Add layer: tf.layers.dot #330
Conversation
src/layers/merge.ts
Outdated
| normalize?: boolean; | ||
| } | ||
|
|
||
| function normalizeAxis(axis: number, dim: number): number { |
There was a problem hiding this comment.
This function gets a positive axis index from a negative one, right? There is a bit of a name collision here with the concept above of 'normlize' meaning l2 normalization. Can we rename this internal function or give a short docstring describing the purpose?
| } | ||
|
|
||
| function normalizeAxis(axis: number, dim: number): number { | ||
| while (axis < 0) { |
There was a problem hiding this comment.
Also, it seems possible that this results in an infinite loop if the user tries to dot product a scalar. Can we throw an error message instead?
src/layers/merge.ts
Outdated
| return axis; | ||
| } | ||
|
|
||
| function batchDot(x: Tensor, y: Tensor, axes: number|number[]): Tensor { |
There was a problem hiding this comment.
should axes type here be number|[number]|[number,number] ? I.e., should lists of length greater than 2 be disallowed?
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 1 approvals obtained (waiting on @caisq, @bileschi, and @nsthorat)
src/layers/merge.ts, line 902 at r1 (raw file):
Previously, bileschi (Stanley Bileschi) wrote…
This function gets a positive axis index from a negative one, right? There is a bit of a name collision here with the concept above of 'normlize' meaning l2 normalization. Can we rename this internal function or give a short docstring describing the purpose?
This method mainly deals with negative axis indices such as -1 and -2. Renamed to interperableAxis and added a doc string.
src/layers/merge.ts, line 903 at r1 (raw file):
Previously, bileschi (Stanley Bileschi) wrote…
Also, it seems possible that this results in an infinite loop if the user tries to dot product a scalar. Can we throw an error message instead?
Good point. Added assertion for rank >= 2 in both x and y.
src/layers/merge.ts, line 909 at r1 (raw file):
Previously, bileschi (Stanley Bileschi) wrote…
should axes type here be number|[number]|[number,number] ? I.e., should lists of length greater than 2 be disallowed?
Done.
|
:!LGTM: |
FEATURE
Fixes tensorflow/tfjs#218
This change is